From 18f770e937ccb21a8417090e7667b6225a22dbe8 Mon Sep 17 00:00:00 2001 From: Pawel Kadluczka Date: Mon, 30 Oct 2017 11:31:57 -0700 Subject: [PATCH] Late parameter binding (#1049) Late parameter binding Storing exception thrown during parameter binding and rethrowing when the method is about to throw. This allows completing invocations with a HubException and keeping the connection open. We will also no longer close the connection if parameters for client side methods cannot be bound. We will log and continue. Fixes: #818 (Also fixing #1005 because I was just touching this line) --- .../HubConnection.cs | 17 +- .../Internal/SignalRClientLoggerExtensions.cs | 12 +- .../Internal/Protocol/InvocationMessage.cs | 35 ++++- .../Internal/Protocol/JsonHubProtocol.cs | 34 +++- .../Protocol/MessagePackHubProtocol.cs | 72 ++++++--- .../DefaultHubLifetimeManager.cs | 2 +- .../RedisHubLifetimeManager.cs | 10 +- .../HttpConnection.cs | 2 +- test/Common/TestClient.cs | 3 +- .../HubConnectionTests.cs | 145 +++++++++++++++++- .../HubConnectionExtensionsTests.cs | 65 ++++---- .../HubConnectionProtocolTests.cs | 2 +- .../Internal/Protocol/JsonHubProtocolTests.cs | 27 ++-- .../Protocol/MessagePackHubProtocolTests.cs | 53 +++++-- .../EndToEndTests.cs | 2 +- 15 files changed, 352 insertions(+), 129 deletions(-) diff --git a/src/Microsoft.AspNetCore.SignalR.Client.Core/HubConnection.cs b/src/Microsoft.AspNetCore.SignalR.Client.Core/HubConnection.cs index f8d1f927e0..5581d94d64 100644 --- a/src/Microsoft.AspNetCore.SignalR.Client.Core/HubConnection.cs +++ b/src/Microsoft.AspNetCore.SignalR.Client.Core/HubConnection.cs @@ -19,7 +19,6 @@ using Microsoft.AspNetCore.Sockets.Features; using Microsoft.AspNetCore.Sockets.Internal; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; -using Newtonsoft.Json; namespace Microsoft.AspNetCore.SignalR.Client { @@ -122,7 +121,7 @@ namespace Microsoft.AspNetCore.SignalR.Client public IDisposable On(string methodName, Type[] parameterTypes, Func handler, object state) { var invocationHandler = new InvocationHandler(parameterTypes, handler, state); - var invocationList = _handlers.AddOrUpdate(methodName, _ => new List { invocationHandler }, + var invocationList = _handlers.AddOrUpdate(methodName, _ => new List { invocationHandler }, (_, invocations) => { lock (invocations) @@ -135,7 +134,7 @@ namespace Microsoft.AspNetCore.SignalR.Client return new Subscription(invocationHandler, invocationList); } - public async Task> StreamAsync(string methodName, Type returnType, object[] args, CancellationToken cancellationToken = default(CancellationToken)) + public async Task> StreamAsync(string methodName, Type returnType, object[] args, CancellationToken cancellationToken = default) { return await StreamAsyncCore(methodName, returnType, args, cancellationToken).ForceAsync(); } @@ -174,7 +173,7 @@ namespace Microsoft.AspNetCore.SignalR.Client return channel; } - public async Task InvokeAsync(string methodName, Type returnType, object[] args, CancellationToken cancellationToken = default(CancellationToken)) => + public async Task InvokeAsync(string methodName, Type returnType, object[] args, CancellationToken cancellationToken = default) => await InvokeAsyncCore(methodName, returnType, args, cancellationToken).ForceAsync(); private async Task InvokeAsyncCore(string methodName, Type returnType, object[] args, CancellationToken cancellationToken) @@ -184,7 +183,7 @@ namespace Microsoft.AspNetCore.SignalR.Client return await task; } - public async Task SendAsync(string methodName, object[] args, CancellationToken cancellationToken = default(CancellationToken)) => + public async Task SendAsync(string methodName, object[] args, CancellationToken cancellationToken = default) => await SendAsyncCore(methodName, args, cancellationToken).ForceAsync(); private Task SendAsyncCore(string methodName, object[] args, CancellationToken cancellationToken) @@ -206,7 +205,8 @@ namespace Microsoft.AspNetCore.SignalR.Client } // Create an invocation descriptor. Client invocations are always blocking - var invocationMessage = new InvocationMessage(irq.InvocationId, nonBlocking, methodName, args); + var invocationMessage = new InvocationMessage(irq.InvocationId, nonBlocking, methodName, + argumentBindingException: null, arguments: args); // We don't need to track invocations for fire an forget calls if (!nonBlocking) @@ -252,7 +252,8 @@ namespace Microsoft.AspNetCore.SignalR.Client switch (message) { case InvocationMessage invocation: - _logger.ReceivedInvocation(invocation.InvocationId, invocation.Target, invocation.Arguments); + _logger.ReceivedInvocation(invocation.InvocationId, invocation.Target, + invocation.ArgumentBindingException != null ? null : invocation.Arguments); await DispatchInvocationAsync(invocation, _connectionActive.Token); break; case CompletionMessage completion: @@ -344,7 +345,7 @@ namespace Microsoft.AspNetCore.SignalR.Client } catch (Exception ex) { - _logger.ExceptionThrownFromCallback(nameof(On), ex); + _logger.ErrorInvokingClientSideMethod(invocation.Target, ex); } } } diff --git a/src/Microsoft.AspNetCore.SignalR.Client.Core/Internal/SignalRClientLoggerExtensions.cs b/src/Microsoft.AspNetCore.SignalR.Client.Core/Internal/SignalRClientLoggerExtensions.cs index 5966fa78cd..e87403c4cc 100644 --- a/src/Microsoft.AspNetCore.SignalR.Client.Core/Internal/SignalRClientLoggerExtensions.cs +++ b/src/Microsoft.AspNetCore.SignalR.Client.Core/Internal/SignalRClientLoggerExtensions.cs @@ -112,8 +112,8 @@ namespace Microsoft.AspNetCore.SignalR.Client.Internal private static readonly Action _streamItemOnNonStreamInvocation = LoggerMessage.Define(LogLevel.Error, new EventId(4, nameof(StreamItemOnNonStreamInvocation)), "Invocation {invocationId} received stream item but was invoked as a non-streamed invocation."); - private static readonly Action _exceptionThrownFromCallback = - LoggerMessage.Define(LogLevel.Error, new EventId(5, nameof(ExceptionThrownFromCallback)), "An exception was thrown from the '{callback}' callback"); + private static readonly Action _errorInvokingClientSideMethod = + LoggerMessage.Define(LogLevel.Error, new EventId(5, nameof(ErrorInvokingClientSideMethod)), "Invoking client side method '{methodName}' failed."); private static readonly Action _receivedUnexpectedMessageTypeForInvokeCompletion = LoggerMessage.Define(LogLevel.Error, new EventId(6, nameof(ReceivedUnexpectedMessageTypeForInvokeCompletion)), "Invocation {invocationId} was invoked as a non-streaming hub method but completed with '{messageType}' message."); @@ -137,7 +137,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Internal { if (logger.IsEnabled(LogLevel.Trace)) { - var argsList = string.Join(", ", args.Select(a => a.GetType().FullName)); + var argsList = args == null ? string.Empty : string.Join(", ", args.Select(a => a?.GetType().FullName ?? "(null)")); _issueInvocation(logger, invocationId, returnType, methodName, argsList, null); } } @@ -161,7 +161,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Internal { if (logger.IsEnabled(LogLevel.Trace)) { - var argsList = string.Join(", ", args.Select(a => a.GetType().FullName)); + var argsList = args == null ? string.Empty : string.Join(", ", args.Select(a => a?.GetType().FullName ?? "(null)")); _receivedInvocation(logger, invocationId, methodName, argsList, null); } } @@ -286,9 +286,9 @@ namespace Microsoft.AspNetCore.SignalR.Client.Internal _streamItemOnNonStreamInvocation(logger, invocationId, null); } - public static void ExceptionThrownFromCallback(this ILogger logger, string callbackName, Exception exception) + public static void ErrorInvokingClientSideMethod(this ILogger logger, string methodName, Exception exception) { - _exceptionThrownFromCallback(logger, callbackName, exception); + _errorInvokingClientSideMethod(logger, methodName, exception); } public static void ReceivedUnexpectedMessageTypeForStreamCompletion(this ILogger logger, string invocationId, string messageType) diff --git a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/InvocationMessage.cs b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/InvocationMessage.cs index 4f8a4c738f..571b1d90bb 100644 --- a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/InvocationMessage.cs +++ b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/InvocationMessage.cs @@ -3,18 +3,42 @@ using System; using System.Linq; +using System.Runtime.ExceptionServices; namespace Microsoft.AspNetCore.SignalR.Internal.Protocol { public class InvocationMessage : HubMessage { + private readonly ExceptionDispatchInfo _argumentBindingException; + private readonly object[] _arguments; + public string Target { get; } - public object[] Arguments { get; } + public object[] Arguments + { + get + { + if (_argumentBindingException != null) + { + _argumentBindingException.Throw(); + } + + return _arguments; + } + } + + public Exception ArgumentBindingException + { + get + { + return _argumentBindingException?.SourceException; + } + } public bool NonBlocking { get; } - public InvocationMessage(string invocationId, bool nonBlocking, string target, params object[] arguments) : base(invocationId) + public InvocationMessage(string invocationId, bool nonBlocking, string target, ExceptionDispatchInfo argumentBindingException, params object[] arguments) + : base(invocationId) { if (string.IsNullOrEmpty(invocationId)) { @@ -26,13 +50,14 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol throw new ArgumentNullException(nameof(target)); } - if (arguments == null) + if ((arguments == null && argumentBindingException == null) || (arguments?.Length > 0 && argumentBindingException != null)) { - throw new ArgumentNullException(nameof(arguments)); + throw new ArgumentException($"'{nameof(argumentBindingException)}' and '{nameof(arguments)}' are mutually exclusive"); } Target = target; - Arguments = arguments; + _arguments = arguments; + _argumentBindingException = argumentBindingException; NonBlocking = nonBlocking; } diff --git a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/JsonHubProtocol.cs b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/JsonHubProtocol.cs index 4643e75f58..6708c80dc8 100644 --- a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/JsonHubProtocol.cs +++ b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/JsonHubProtocol.cs @@ -3,8 +3,8 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.IO; +using System.Runtime.ExceptionServices; using Microsoft.AspNetCore.SignalR.Internal.Formatters; using Newtonsoft.Json; using Newtonsoft.Json.Linq; @@ -242,22 +242,40 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol var args = JsonUtils.GetRequiredProperty(json, ArgumentsPropertyName, JTokenType.Array); var paramTypes = binder.GetParameterTypes(target); + + try + { + var arguments = BindArguments(args, paramTypes); + return new InvocationMessage(invocationId, nonBlocking, target, argumentBindingException: null, arguments: arguments); + } + catch (Exception ex) + { + return new InvocationMessage(invocationId, nonBlocking, target, ExceptionDispatchInfo.Capture(ex)); + } + } + + private object[] BindArguments(JArray args, Type[] paramTypes) + { var arguments = new object[args.Count]; if (paramTypes.Length != arguments.Length) { throw new FormatException($"Invocation provides {arguments.Length} argument(s) but target expects {paramTypes.Length}."); } - for (var i = 0; i < paramTypes.Length; i++) + try { - var paramType = paramTypes[i]; + for (var i = 0; i < paramTypes.Length; i++) + { + var paramType = paramTypes[i]; + arguments[i] = args[i].ToObject(paramType, _payloadSerializer); + } - // TODO(anurse): We can add some DI magic here to allow users to provide their own serialization - // Related Bug: https://github.com/aspnet/SignalR/issues/261 - arguments[i] = args[i].ToObject(paramType, _payloadSerializer); + return arguments; + } + catch (Exception ex) + { + throw new FormatException("Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.", ex); } - - return new InvocationMessage(invocationId, nonBlocking, target, arguments); } private StreamItemMessage BindResultMessage(JObject json, IInvocationBinder binder) diff --git a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/MessagePackHubProtocol.cs b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/MessagePackHubProtocol.cs index be3ed3a8f7..794a1bf34c 100644 --- a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/MessagePackHubProtocol.cs +++ b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/MessagePackHubProtocol.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; using System.IO; +using System.Runtime.ExceptionServices; using Microsoft.AspNetCore.SignalR.Internal.Formatters; using MsgPack; using MsgPack.Serialization; @@ -55,24 +56,26 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol private static HubMessage ParseMessage(Stream input, IInvocationBinder binder) { - var unpacker = Unpacker.Create(input); - var arraySize = ReadArrayLength(unpacker, "elementCount"); - var messageType = ReadInt32(unpacker, "messageType"); - - switch (messageType) + using (var unpacker = Unpacker.Create(input)) { - case InvocationMessageType: - return CreateInvocationMessage(unpacker, binder); - case StreamItemMessageType: - return CreateStreamItemMessage(unpacker, binder); - case CompletionMessageType: - return CreateCompletionMessage(unpacker, binder); - case StreamCompletionMessageType: - return CreateStreamCompletionMessage(unpacker, arraySize, binder); - case CancelInvocationMessageType: - return CreateCancelInvocationMessage(unpacker); - default: - throw new FormatException($"Invalid message type: {messageType}."); + var arraySize = ReadArrayLength(unpacker, "elementCount"); + var messageType = ReadInt32(unpacker, "messageType"); + + switch (messageType) + { + case InvocationMessageType: + return CreateInvocationMessage(unpacker, binder); + case StreamItemMessageType: + return CreateStreamItemMessage(unpacker, binder); + case CompletionMessageType: + return CreateCompletionMessage(unpacker, binder); + case StreamCompletionMessageType: + return CreateStreamCompletionMessage(unpacker, arraySize, binder); + case CancelInvocationMessageType: + return CreateCancelInvocationMessage(unpacker); + default: + throw new FormatException($"Invalid message type: {messageType}."); + } } } @@ -81,22 +84,43 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol var invocationId = ReadInvocationId(unpacker); var nonBlocking = ReadBoolean(unpacker, "nonBlocking"); var target = ReadString(unpacker, "target"); - var argumentCount = ReadArrayLength(unpacker, "arguments"); var parameterTypes = binder.GetParameterTypes(target); + try + { + var arguments = BindArguments(unpacker, parameterTypes); + return new InvocationMessage(invocationId, nonBlocking, target, argumentBindingException: null, arguments: arguments); + } + catch (Exception ex) + { + return new InvocationMessage(invocationId, nonBlocking, target, ExceptionDispatchInfo.Capture(ex)); + } + } + + private static object[] BindArguments(Unpacker unpacker, Type[] parameterTypes) + { + var argumentCount = ReadArrayLength(unpacker, "arguments"); + if (parameterTypes.Length != argumentCount) { throw new FormatException( - $"Target method expects {parameterTypes.Length} arguments(s) but invocation has {argumentCount} argument(s)."); + $"Invocation provides {argumentCount} argument(s) but target expects {parameterTypes.Length}."); } - var arguments = new object[argumentCount]; - for (var i = 0; i < argumentCount; i++) + try { - arguments[i] = DeserializeObject(unpacker, parameterTypes[i], "argument"); - } + var arguments = new object[argumentCount]; + for (var i = 0; i < argumentCount; i++) + { + arguments[i] = DeserializeObject(unpacker, parameterTypes[i], "argument"); + } - return new InvocationMessage(invocationId, nonBlocking, target, arguments); + return arguments; + } + catch (Exception ex) + { + throw new FormatException("Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.", ex); + } } private static StreamItemMessage CreateStreamItemMessage(Unpacker unpacker, IInvocationBinder binder) diff --git a/src/Microsoft.AspNetCore.SignalR.Core/DefaultHubLifetimeManager.cs b/src/Microsoft.AspNetCore.SignalR.Core/DefaultHubLifetimeManager.cs index fcad6dd4a6..b4051dfc9b 100644 --- a/src/Microsoft.AspNetCore.SignalR.Core/DefaultHubLifetimeManager.cs +++ b/src/Microsoft.AspNetCore.SignalR.Core/DefaultHubLifetimeManager.cs @@ -125,7 +125,7 @@ namespace Microsoft.AspNetCore.SignalR private InvocationMessage CreateInvocationMessage(string methodName, object[] args) { - return new InvocationMessage(GetInvocationId(), nonBlocking: true, target: methodName, arguments: args); + return new InvocationMessage(GetInvocationId(), nonBlocking: true, target: methodName, argumentBindingException: null, arguments: args); } public override Task InvokeUserAsync(string userId, string methodName, object[] args) diff --git a/src/Microsoft.AspNetCore.SignalR.Redis/RedisHubLifetimeManager.cs b/src/Microsoft.AspNetCore.SignalR.Redis/RedisHubLifetimeManager.cs index 2e714a4606..e0c6c6ae43 100644 --- a/src/Microsoft.AspNetCore.SignalR.Redis/RedisHubLifetimeManager.cs +++ b/src/Microsoft.AspNetCore.SignalR.Redis/RedisHubLifetimeManager.cs @@ -185,7 +185,7 @@ namespace Microsoft.AspNetCore.SignalR.Redis public override Task InvokeAllAsync(string methodName, object[] args) { - var message = new InvocationMessage(GetInvocationId(), nonBlocking: true, target: methodName, arguments: args); + var message = new InvocationMessage(GetInvocationId(), nonBlocking: true, target: methodName, argumentBindingException: null, arguments: args); return PublishAsync(_channelNamePrefix, message); } @@ -203,7 +203,7 @@ namespace Microsoft.AspNetCore.SignalR.Redis throw new ArgumentNullException(nameof(connectionId)); } - var message = new InvocationMessage(GetInvocationId(), nonBlocking: true, target: methodName, arguments: args); + var message = new InvocationMessage(GetInvocationId(), nonBlocking: true, target: methodName, argumentBindingException: null, arguments: args); // If the connection is local we can skip sending the message through the bus since we require sticky connections. // This also saves serializing and deserializing the message! @@ -223,14 +223,14 @@ namespace Microsoft.AspNetCore.SignalR.Redis throw new ArgumentNullException(nameof(groupName)); } - var message = new InvocationMessage(GetInvocationId(), nonBlocking: true, target: methodName, arguments: args); + var message = new InvocationMessage(GetInvocationId(), nonBlocking: true, target: methodName, argumentBindingException: null, arguments: args); return PublishAsync(_channelNamePrefix + ".group." + groupName, message); } public override Task InvokeUserAsync(string userId, string methodName, object[] args) { - var message = new InvocationMessage(GetInvocationId(), nonBlocking: true, target: methodName, arguments: args); + var message = new InvocationMessage(GetInvocationId(), nonBlocking: true, target: methodName, argumentBindingException: null, arguments: args); return PublishAsync(_channelNamePrefix + ".user." + userId, message); } @@ -556,7 +556,7 @@ namespace Microsoft.AspNetCore.SignalR.Redis public IReadOnlyList ExcludedIds; public RedisExcludeClientsMessage(string invocationId, bool nonBlocking, string target, IReadOnlyList excludedIds, params object[] arguments) - : base(invocationId, nonBlocking, target, arguments) + : base(invocationId, nonBlocking, target, argumentBindingException: null, arguments: arguments) { ExcludedIds = excludedIds; } diff --git a/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.cs b/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.cs index 2bc0ac6c49..ab4eafb255 100644 --- a/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.cs +++ b/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.cs @@ -391,7 +391,7 @@ namespace Microsoft.AspNetCore.Sockets.Client _logger.EndReceive(_connectionId); } - public async Task SendAsync(byte[] data, CancellationToken cancellationToken = default(CancellationToken)) => + public async Task SendAsync(byte[] data, CancellationToken cancellationToken = default) => await SendAsyncCore(data, cancellationToken).ForceAsync(); private async Task SendAsyncCore(byte[] data, CancellationToken cancellationToken) diff --git a/test/Common/TestClient.cs b/test/Common/TestClient.cs index 1a4aa11b4a..2f866a42d1 100644 --- a/test/Common/TestClient.cs +++ b/test/Common/TestClient.cs @@ -136,7 +136,8 @@ namespace Microsoft.AspNetCore.SignalR.Tests public Task SendInvocationAsync(string methodName, bool nonBlocking, params object[] args) { var invocationId = GetInvocationId(); - return SendHubMessageAsync(new InvocationMessage(invocationId, nonBlocking, methodName, args)); + return SendHubMessageAsync(new InvocationMessage(invocationId, nonBlocking, methodName, + argumentBindingException: null, arguments: args)); } public async Task SendHubMessageAsync(HubMessage message) diff --git a/test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs b/test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs index 0c45e36243..42c8380be1 100644 --- a/test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs @@ -323,7 +323,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests [Theory] [MemberData(nameof(HubProtocolsAndTransportsAndHubPaths))] - public async Task ServerClosesConnectionIfHubMethodCannotBeResolved(IHubProtocol hubProtocol, TransportType transportType, string hubPath) + public async Task ServerThrowsHubExceptionIfHubMethodCannotBeResolved(IHubProtocol hubProtocol, TransportType transportType, string hubPath) { using (StartLog(out var loggerFactory)) { @@ -333,9 +333,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests { await connection.StartAsync().OrTimeout(); - var ex = await Assert.ThrowsAnyAsync( - async () => await connection.InvokeAsync("!@#$%")).OrTimeout(); - + var ex = await Assert.ThrowsAsync(() => connection.InvokeAsync("!@#$%")).OrTimeout(); Assert.Equal("Unknown hub method '!@#$%'", ex.Message); } catch (Exception ex) @@ -350,6 +348,145 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests } } + [Theory] + [MemberData(nameof(HubProtocolsAndTransportsAndHubPaths))] + public async Task ServerThrowsHubExceptionOnHubMethodArgumentCountMismatch(IHubProtocol hubProtocol, TransportType transportType, string hubPath) + { + using (StartLog(out var loggerFactory)) + { + var httpConnection = new HttpConnection(new Uri(_serverFixture.BaseUrl + hubPath), transportType, loggerFactory); + var connection = new HubConnection(httpConnection, hubProtocol, loggerFactory); + try + { + await connection.StartAsync().OrTimeout(); + + var ex = await Assert.ThrowsAsync(() => connection.InvokeAsync("Echo", "p1", 42)).OrTimeout(); + Assert.Equal("Invocation provides 2 argument(s) but target expects 1.", ex.Message); + } + catch (Exception ex) + { + loggerFactory.CreateLogger().LogError(ex, "Exception from test"); + throw; + } + finally + { + await connection.DisposeAsync().OrTimeout(); + } + } + } + + [Theory] + [MemberData(nameof(HubProtocolsAndTransportsAndHubPaths))] + public async Task ServerThrowsHubExceptionOnHubMethodArgumentTypeMismatch(IHubProtocol hubProtocol, TransportType transportType, string hubPath) + { + using (StartLog(out var loggerFactory)) + { + var httpConnection = new HttpConnection(new Uri(_serverFixture.BaseUrl + hubPath), transportType, loggerFactory); + var connection = new HubConnection(httpConnection, hubProtocol, loggerFactory); + try + { + await connection.StartAsync().OrTimeout(); + + var ex = await Assert.ThrowsAsync(() => connection.InvokeAsync("Echo", new int[] { 42 })).OrTimeout(); + Assert.StartsWith("Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.", ex.Message); + } + catch (Exception ex) + { + loggerFactory.CreateLogger().LogError(ex, "Exception from test"); + throw; + } + finally + { + await connection.DisposeAsync().OrTimeout(); + } + } + } + + [Theory(Skip="https://github.com/aspnet/SignalR/issues/1053")] + [MemberData(nameof(HubProtocolsAndTransportsAndHubPaths))] + public async Task ServerThrowsHubExceptionIfStreamingHubMethodCannotBeResolved(IHubProtocol hubProtocol, TransportType transportType, string hubPath) + { + using (StartLog(out var loggerFactory)) + { + var httpConnection = new HttpConnection(new Uri(_serverFixture.BaseUrl + hubPath), transportType, loggerFactory); + var connection = new HubConnection(httpConnection, hubProtocol, loggerFactory); + try + { + await connection.StartAsync().OrTimeout(); + + var channel = await connection.StreamAsync("!@#$%"); + var ex = await Assert.ThrowsAsync(() => channel.ReadAllAsync().OrTimeout()); + Assert.Equal("Unknown hub method '!@#$%'", ex.Message); + } + catch (Exception ex) + { + loggerFactory.CreateLogger().LogError(ex, "Exception from test"); + throw; + } + finally + { + await connection.DisposeAsync().OrTimeout(); + } + } + } + + [Theory] + [MemberData(nameof(HubProtocolsAndTransportsAndHubPaths))] + public async Task ServerThrowsHubExceptionOnStreamingHubMethodArgumentCountMismatch(IHubProtocol hubProtocol, TransportType transportType, string hubPath) + { + using (StartLog(out var loggerFactory)) + { + loggerFactory.AddConsole(LogLevel.Trace); + var httpConnection = new HttpConnection(new Uri(_serverFixture.BaseUrl + hubPath), transportType, loggerFactory); + var connection = new HubConnection(httpConnection, hubProtocol, loggerFactory); + try + { + await connection.StartAsync().OrTimeout(); + + var channel = await connection.StreamAsync("Stream", 42, 42); + var ex = await Assert.ThrowsAsync(() => channel.ReadAllAsync().OrTimeout()); + Assert.Equal("Invocation provides 2 argument(s) but target expects 1.", ex.Message); + } + catch (Exception ex) + { + loggerFactory.CreateLogger().LogError(ex, "Exception from test"); + throw; + } + finally + { + await connection.DisposeAsync().OrTimeout(); + } + } + } + + [Theory] + [MemberData(nameof(HubProtocolsAndTransportsAndHubPaths))] + public async Task ServerThrowsHubExceptionOnStreamingHubMethodArgumentTypeMismatch(IHubProtocol hubProtocol, TransportType transportType, string hubPath) + { + using (StartLog(out var loggerFactory)) + { + var httpConnection = new HttpConnection(new Uri(_serverFixture.BaseUrl + hubPath), transportType, loggerFactory); + var connection = new HubConnection(httpConnection, hubProtocol, loggerFactory); + try + { + await connection.StartAsync().OrTimeout(); + + var channel = await connection.StreamAsync("Stream", "xyz"); + var ex = await Assert.ThrowsAsync(() => channel.ReadAllAsync().OrTimeout()); + Assert.StartsWith("Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.", ex.Message); + } + catch (Exception ex) + { + loggerFactory.CreateLogger().LogError(ex, "Exception from test"); + throw; + } + finally + { + await connection.DisposeAsync().OrTimeout(); + } + } + } + public static IEnumerable HubProtocolsAndTransportsAndHubPaths { get diff --git a/test/Microsoft.AspNetCore.SignalR.Client.Tests/HubConnectionExtensionsTests.cs b/test/Microsoft.AspNetCore.SignalR.Client.Tests/HubConnectionExtensionsTests.cs index 34e0a4b9ae..3b5b70dde8 100644 --- a/test/Microsoft.AspNetCore.SignalR.Client.Tests/HubConnectionExtensionsTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Client.Tests/HubConnectionExtensionsTests.cs @@ -123,28 +123,16 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests } [Fact] - public async Task ConnectionClosedOnCallbackArgumentCountMismatch() + public async Task ConnectionNotClosedOnCallbackArgumentCountMismatch() { var connection = new TestConnection(); var hubConnection = new HubConnection(connection, new JsonHubProtocol(new JsonSerializer()), new LoggerFactory()); - var closeTcs = new TaskCompletionSource(); - hubConnection.Closed += e => - { - if (e == null) - { - closeTcs.TrySetResult(null); - } - else - { - closeTcs.TrySetException(e); - } - return Task.CompletedTask; - }; + var receiveTcs = new TaskCompletionSource(); try { - hubConnection.On("Foo", r => { }); - await hubConnection.StartAsync(); + hubConnection.On("Foo", r => { receiveTcs.SetResult(r); }); + await hubConnection.StartAsync().OrTimeout(); await connection.ReceiveJsonMessage( new @@ -155,39 +143,34 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests arguments = new object[] { 42, "42" } }).OrTimeout(); - var ex = await Assert.ThrowsAsync(async () => await closeTcs.Task.OrTimeout()); - Assert.Equal("Invocation provides 2 argument(s) but target expects 1.", ex.Message); + await connection.ReceiveJsonMessage( + new + { + invocationId = "2", + type = 1, + target = "Foo", + arguments = new object[] { 42 } + }).OrTimeout(); + + Assert.Equal(42, await receiveTcs.Task.OrTimeout()); } finally { await hubConnection.DisposeAsync().OrTimeout(); - await connection.DisposeAsync().OrTimeout(); } } [Fact] - public async Task ConnectionClosedOnCallbackArgumentTypeMismatch() + public async Task ConnectionNotClosedOnCallbackArgumentTypeMismatch() { var connection = new TestConnection(); var hubConnection = new HubConnection(connection, new JsonHubProtocol(), new LoggerFactory()); - var closeTcs = new TaskCompletionSource(); - hubConnection.Closed += e => - { - if (e == null) - { - closeTcs.TrySetResult(null); - } - else - { - closeTcs.TrySetException(e); - } - return Task.CompletedTask; - }; + var receiveTcs = new TaskCompletionSource(); try { - hubConnection.On("Foo", r => { }); - await hubConnection.StartAsync(); + hubConnection.On("Foo", r => { receiveTcs.SetResult(r); }); + await hubConnection.StartAsync().OrTimeout(); await connection.ReceiveJsonMessage( new @@ -198,12 +181,20 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests arguments = new object[] { "xxx" } }).OrTimeout(); - var ex = await Assert.ThrowsAsync(async () => await closeTcs.Task.OrTimeout()); + await connection.ReceiveJsonMessage( + new + { + invocationId = "2", + type = 1, + target = "Foo", + arguments = new object[] { 42 } + }).OrTimeout(); + + Assert.Equal(42, await receiveTcs.Task.OrTimeout()); } finally { await hubConnection.DisposeAsync().OrTimeout(); - await connection.DisposeAsync().OrTimeout(); } } } diff --git a/test/Microsoft.AspNetCore.SignalR.Client.Tests/HubConnectionProtocolTests.cs b/test/Microsoft.AspNetCore.SignalR.Client.Tests/HubConnectionProtocolTests.cs index 72e462dae9..3d93cd9afb 100644 --- a/test/Microsoft.AspNetCore.SignalR.Client.Tests/HubConnectionProtocolTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Client.Tests/HubConnectionProtocolTests.cs @@ -443,7 +443,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests using (var ms = new MemoryStream()) { new MessagePackHubProtocol() - .WriteMessage(new InvocationMessage("1", true, "MyMethod", 42), ms); + .WriteMessage(new InvocationMessage("1", true, "MyMethod", null, 42), ms); var invokeMessage = Convert.ToBase64String(ms.ToArray()); var payloadSize = invokeMessage.Length.ToString(CultureInfo.InvariantCulture); diff --git a/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/JsonHubProtocolTests.cs b/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/JsonHubProtocolTests.cs index ecf971530b..03997878ec 100644 --- a/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/JsonHubProtocolTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/JsonHubProtocolTests.cs @@ -17,14 +17,14 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol { public static IEnumerable ProtocolTestData => new[] { - new object[] { new InvocationMessage("123", true, "Target", 1, "Foo", 2.0f), true, NullValueHandling.Ignore, "{\"invocationId\":\"123\",\"type\":1,\"target\":\"Target\",\"nonBlocking\":true,\"arguments\":[1,\"Foo\",2.0]}" }, - new object[] { new InvocationMessage("123", false, "Target", 1, "Foo", 2.0f), true, NullValueHandling.Ignore, "{\"invocationId\":\"123\",\"type\":1,\"target\":\"Target\",\"arguments\":[1,\"Foo\",2.0]}" }, - new object[] { new InvocationMessage("123", false, "Target", true), true, NullValueHandling.Ignore, "{\"invocationId\":\"123\",\"type\":1,\"target\":\"Target\",\"arguments\":[true]}" }, - new object[] { new InvocationMessage("123", false, "Target", new object[] { null }), true, NullValueHandling.Ignore, "{\"invocationId\":\"123\",\"type\":1,\"target\":\"Target\",\"arguments\":[null]}" }, - new object[] { new InvocationMessage("123", false, "Target", new CustomObject()), false, NullValueHandling.Ignore, "{\"invocationId\":\"123\",\"type\":1,\"target\":\"Target\",\"arguments\":[{\"StringProp\":\"SignalR!\",\"DoubleProp\":6.2831853071,\"IntProp\":42,\"DateTimeProp\":\"2017-04-11T00:00:00\",\"ByteArrProp\":\"AQID\"}]}" }, - new object[] { new InvocationMessage("123", false, "Target", new CustomObject()), true, NullValueHandling.Ignore, "{\"invocationId\":\"123\",\"type\":1,\"target\":\"Target\",\"arguments\":[{\"stringProp\":\"SignalR!\",\"doubleProp\":6.2831853071,\"intProp\":42,\"dateTimeProp\":\"2017-04-11T00:00:00\",\"byteArrProp\":\"AQID\"}]}" }, - new object[] { new InvocationMessage("123", false, "Target", new CustomObject()), false, NullValueHandling.Include, "{\"invocationId\":\"123\",\"type\":1,\"target\":\"Target\",\"arguments\":[{\"StringProp\":\"SignalR!\",\"DoubleProp\":6.2831853071,\"IntProp\":42,\"DateTimeProp\":\"2017-04-11T00:00:00\",\"NullProp\":null,\"ByteArrProp\":\"AQID\"}]}" }, - new object[] { new InvocationMessage("123", false, "Target", new CustomObject()), true, NullValueHandling.Include, "{\"invocationId\":\"123\",\"type\":1,\"target\":\"Target\",\"arguments\":[{\"stringProp\":\"SignalR!\",\"doubleProp\":6.2831853071,\"intProp\":42,\"dateTimeProp\":\"2017-04-11T00:00:00\",\"nullProp\":null,\"byteArrProp\":\"AQID\"}]}" }, + new object[] { new InvocationMessage("123", true, "Target", null, 1, "Foo", 2.0f), true, NullValueHandling.Ignore, "{\"invocationId\":\"123\",\"type\":1,\"target\":\"Target\",\"nonBlocking\":true,\"arguments\":[1,\"Foo\",2.0]}" }, + new object[] { new InvocationMessage("123", false, "Target", null, 1, "Foo", 2.0f), true, NullValueHandling.Ignore, "{\"invocationId\":\"123\",\"type\":1,\"target\":\"Target\",\"arguments\":[1,\"Foo\",2.0]}" }, + new object[] { new InvocationMessage("123", false, "Target", null, true), true, NullValueHandling.Ignore, "{\"invocationId\":\"123\",\"type\":1,\"target\":\"Target\",\"arguments\":[true]}" }, + new object[] { new InvocationMessage("123", false, "Target", null, new object[] { null }), true, NullValueHandling.Ignore, "{\"invocationId\":\"123\",\"type\":1,\"target\":\"Target\",\"arguments\":[null]}" }, + new object[] { new InvocationMessage("123", false, "Target", null, new CustomObject()), false, NullValueHandling.Ignore, "{\"invocationId\":\"123\",\"type\":1,\"target\":\"Target\",\"arguments\":[{\"StringProp\":\"SignalR!\",\"DoubleProp\":6.2831853071,\"IntProp\":42,\"DateTimeProp\":\"2017-04-11T00:00:00\",\"ByteArrProp\":\"AQID\"}]}" }, + new object[] { new InvocationMessage("123", false, "Target", null, new CustomObject()), true, NullValueHandling.Ignore, "{\"invocationId\":\"123\",\"type\":1,\"target\":\"Target\",\"arguments\":[{\"stringProp\":\"SignalR!\",\"doubleProp\":6.2831853071,\"intProp\":42,\"dateTimeProp\":\"2017-04-11T00:00:00\",\"byteArrProp\":\"AQID\"}]}" }, + new object[] { new InvocationMessage("123", false, "Target", null, new CustomObject()), false, NullValueHandling.Include, "{\"invocationId\":\"123\",\"type\":1,\"target\":\"Target\",\"arguments\":[{\"StringProp\":\"SignalR!\",\"DoubleProp\":6.2831853071,\"IntProp\":42,\"DateTimeProp\":\"2017-04-11T00:00:00\",\"NullProp\":null,\"ByteArrProp\":\"AQID\"}]}" }, + new object[] { new InvocationMessage("123", false, "Target", null, new CustomObject()), true, NullValueHandling.Include, "{\"invocationId\":\"123\",\"type\":1,\"target\":\"Target\",\"arguments\":[{\"stringProp\":\"SignalR!\",\"doubleProp\":6.2831853071,\"intProp\":42,\"dateTimeProp\":\"2017-04-11T00:00:00\",\"nullProp\":null,\"byteArrProp\":\"AQID\"}]}" }, new object[] { new StreamItemMessage("123", 1), true, NullValueHandling.Ignore, "{\"invocationId\":\"123\",\"type\":2,\"item\":1}" }, new object[] { new StreamItemMessage("123", "Foo"), true, NullValueHandling.Ignore, "{\"invocationId\":\"123\",\"type\":2,\"item\":\"Foo\"}" }, @@ -114,6 +114,9 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol [InlineData("{'type':9}", "Unknown message type: 9")] [InlineData("{'type':'foo'}", "Expected 'type' to be of type Integer.")] + + [InlineData("{'type':1,'invocationId':'42','target':'foo','arguments':[42, 'foo'],'nonBlocking':42}", "Expected 'nonBlocking' to be of type Boolean.")] + [InlineData("{'type':3,'invocationId':'42','error':'foo','result':true}", "The 'error' and 'result' properties are mutually exclusive.")] public void InvalidMessages(string input, string expectedMessage) { input = Frame(input); @@ -126,15 +129,15 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol [Theory] [InlineData("{'type':1,'invocationId':'42','target':'foo','arguments':[]}", "Invocation provides 0 argument(s) but target expects 2.")] - [InlineData("{'type':1,'invocationId':'42','target':'foo','arguments':[42, 'foo'],'nonBlocking':42}", "Expected 'nonBlocking' to be of type Boolean.")] - [InlineData("{'type':3,'invocationId':'42','error':'foo','result':true}", "The 'error' and 'result' properties are mutually exclusive.")] - public void InvalidMessagesWithBinder(string input, string expectedMessage) + [InlineData("{'type':1,'invocationId':'42','target':'foo','arguments':[ 'abc', 'xyz']}", "Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.")] + public void ArgumentBindingErrors(string input, string expectedMessage) { input = Frame(input); var binder = new TestBinder(paramTypes: new[] { typeof(int), typeof(string) }, returnType: typeof(bool)); var protocol = new JsonHubProtocol(); - var ex = Assert.Throws(() => protocol.TryParseMessages(Encoding.UTF8.GetBytes(input), binder, out var messages)); + protocol.TryParseMessages(Encoding.UTF8.GetBytes(input), binder, out var messages); + var ex = Assert.Throws(() => ((InvocationMessage)messages[0]).Arguments); Assert.Equal(expectedMessage, ex.Message); } diff --git a/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/MessagePackHubProtocolTests.cs b/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/MessagePackHubProtocolTests.cs index c65ee54f52..ba4242c5ca 100644 --- a/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/MessagePackHubProtocolTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/MessagePackHubProtocolTests.cs @@ -17,13 +17,13 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol public static IEnumerable TestMessages => new[] { - new object[] { new[] { new InvocationMessage("xyz", /*nonBlocking*/ false, "method") } }, - new object[] { new[] { new InvocationMessage("xyz", /*nonBlocking*/ true, "method") } }, - new object[] { new[] { new InvocationMessage("xyz", /*nonBlocking*/ true, "method", new object[] { null }) } }, - new object[] { new[] { new InvocationMessage("xyz", /*nonBlocking*/ true, "method", 42) } }, - new object[] { new[] { new InvocationMessage("xyz", /*nonBlocking*/ true, "method", 42, "string") } }, - new object[] { new[] { new InvocationMessage("xyz", /*nonBlocking*/ true, "method", 42, "string", new CustomObject()) } }, - new object[] { new[] { new InvocationMessage("xyz", /*nonBlocking*/ true, "method", new[] { new CustomObject(), new CustomObject() }) } }, + new object[] { new[] { new InvocationMessage("xyz", /*nonBlocking*/ false, "method", null) } }, + new object[] { new[] { new InvocationMessage("xyz", /*nonBlocking*/ true, "method", null) } }, + new object[] { new[] { new InvocationMessage("xyz", /*nonBlocking*/ true, "method", null, new object[] { null }) } }, + new object[] { new[] { new InvocationMessage("xyz", /*nonBlocking*/ true, "method", null, 42) } }, + new object[] { new[] { new InvocationMessage("xyz", /*nonBlocking*/ true, "method", null, 42, "string") } }, + new object[] { new[] { new InvocationMessage("xyz", /*nonBlocking*/ true, "method", null, 42, "string", new CustomObject()) } }, + new object[] { new[] { new InvocationMessage("xyz", /*nonBlocking*/ true, "method", null, new[] { new CustomObject(), new CustomObject() }) } }, new object[] { new[] { new CompletionMessage("xyz", error: "Error not found!", result: null, hasResult: false) } }, new object[] { new[] { new CompletionMessage("xyz", error: null, result: null, hasResult: false) } }, @@ -52,7 +52,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol { new HubMessage[] { - new InvocationMessage("xyz", /*nonBlocking*/ true, "method", 42, "string", new CustomObject()), + new InvocationMessage("xyz", /*nonBlocking*/ true, "method", null, 42, "string", new CustomObject()), new CompletionMessage("xyz", error: null, result: 42, hasResult: true), new StreamItemMessage("xyz", null), new CompletionMessage("xyz", error: null, result: new CustomObject(), hasResult: true), @@ -93,12 +93,6 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol new object[] { new byte[] { 0x95, 0x01, 0xa3, 0x78, 0x79, 0x7a, 0xc2 }, "Reading 'target' as String failed." }, // target missing new object[] { new byte[] { 0x95, 0x01, 0xa3, 0x78, 0x79, 0x7a, 0xc2, 0x00 }, "Reading 'target' as String failed." }, // 0x00 is Int new object[] { new byte[] { 0x95, 0x01, 0xa3, 0x78, 0x79, 0x7a, 0xc2, 0xa1 }, "Reading 'target' as String failed." }, // string is cut - new object[] { new byte[] { 0x95, 0x01, 0xa3, 0x78, 0x79, 0x7a, 0xc2, 0xa1, 0x78 }, "Reading array length for 'arguments' failed." }, // array is missing - new object[] { new byte[] { 0x95, 0x01, 0xa3, 0x78, 0x79, 0x7a, 0xc2, 0xa1, 0x78, 0x00 }, "Reading array length for 'arguments' failed." }, // 0x00 is not array marker - new object[] { new byte[] { 0x95, 0x01, 0xa3, 0x78, 0x79, 0x7a, 0xc2, 0xa1, 0x78, 0x91 }, "Deserializing object of the `String` type for 'argument' failed." }, // array is missing elements - new object[] { new byte[] { 0x95, 0x01, 0xa3, 0x78, 0x79, 0x7a, 0xc2, 0xa1, 0x78, 0x91, 0xa2, 0x78 }, "Deserializing object of the `String` type for 'argument' failed." }, // array element is cut - new object[] { new byte[] { 0x95, 0x01, 0xa3, 0x78, 0x79, 0x7a, 0xc2, 0xa1, 0x78, 0x92, 0xa0, 0x00 }, "Target method expects 1 arguments(s) but invocation has 2 argument(s)." }, // argument count does not match binder argument count - new object[] { new byte[] { 0x95, 0x01, 0xa3, 0x78, 0x79, 0x7a, 0xc2, 0xa1, 0x78, 0x91, 0x00 }, "Deserializing object of the `String` type for 'argument' failed." }, // argument type mismatch // StreamItemMessage new object[] { new byte[] { 0x93, 0x02 }, "Reading 'invocationId' as String failed." }, // 0xc2 is Bool false @@ -136,6 +130,35 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol Assert.Equal(expectedExceptionMessage, exception.Message); } + public static IEnumerable ArgumentBindingErrors => new[] + { + new object[] { new byte[] { 0x95, 0x01, 0xa3, 0x78, 0x79, 0x7a, 0xc2, 0xa1, 0x78 }, "Reading array length for 'arguments' failed." }, // array is missing + new object[] { new byte[] { 0x95, 0x01, 0xa3, 0x78, 0x79, 0x7a, 0xc2, 0xa1, 0x78, 0x00 }, "Reading array length for 'arguments' failed." }, // 0x00 is not array marker + new object[] { new byte[] { 0x95, 0x01, 0xa3, 0x78, 0x79, 0x7a, 0xc2, 0xa1, 0x78, 0x91 }, "Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked." }, // array is missing elements + new object[] { new byte[] { 0x95, 0x01, 0xa3, 0x78, 0x79, 0x7a, 0xc2, 0xa1, 0x78, 0x91, 0xa2, 0x78 }, "Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked." }, // array element is cut + new object[] { new byte[] { 0x95, 0x01, 0xa3, 0x78, 0x79, 0x7a, 0xc2, 0xa1, 0x78, 0x92, 0xa0, 0x00 }, "Invocation provides 2 argument(s) but target expects 1." }, // argument count does not match binder argument count + new object[] { new byte[] { 0x95, 0x01, 0xa3, 0x78, 0x79, 0x7a, 0xc2, 0xa1, 0x78, 0x91, 0x00 }, "Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked." }, // argument type mismatch + }; + + [Theory] + [MemberData(nameof(ArgumentBindingErrors))] + public void GettingArgumentsThrowsIfBindingFailed(byte[] payload, string expectedExceptionMessage) + { + var payloadSize = payload.Length; + Debug.Assert(payloadSize <= 0x7f, "This test does not support payloads larger than 127 bytes"); + + // prefix payload with the size + var buffer = new byte[1 + payloadSize]; + buffer[0] = (byte)(payloadSize & 0x7f); + Array.Copy(payload, 0, buffer, 1, payloadSize); + + var binder = new TestBinder(new[] { typeof(string) }, typeof(string)); + _hubProtocol.TryParseMessages(buffer, binder, out var messages); + var exception = Assert.Throws(() => ((InvocationMessage)messages[0]).Arguments); + + Assert.Equal(expectedExceptionMessage, exception.Message); + } + [Theory] [InlineData(new object[] { new byte[] { 0x05, 0x01 }, 0 })] [InlineData(new object[] { @@ -157,7 +180,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol { new object[] { - new InvocationMessage("0", false, "A", 1, new CustomObject()), + new InvocationMessage("0", false, "A", null, 1, new CustomObject()), new byte[] { 0x6c, 0x95, 0x01, 0xa1, 0x30, 0xc2, 0xa1, 0x41, diff --git a/test/Microsoft.AspNetCore.SignalR.Tests/EndToEndTests.cs b/test/Microsoft.AspNetCore.SignalR.Tests/EndToEndTests.cs index 326e15e855..4917d82394 100644 --- a/test/Microsoft.AspNetCore.SignalR.Tests/EndToEndTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Tests/EndToEndTests.cs @@ -103,7 +103,7 @@ namespace Microsoft.AspNetCore.SignalR.Tests [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))) + using (StartLog(out var loggerFactory)) { var logger = loggerFactory.CreateLogger(); var url = _serverFixture.BaseUrl + "/echo";