Skip to content

[SignalR] Add parameter index to invocation binding exception message #64016

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[SignalR] Add parameter index to invocation binding exception message #64016

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1244,7 +1244,7 @@ public async Task ServerThrowsHubExceptionOnStreamingHubMethodArgumentTypeMismat

var channel = await connection.StreamAsChannelAsync<int>("Stream", "xyz");
var ex = await Assert.ThrowsAsync<HubException>(() => channel.ReadAndCollectAllAsync().DefaultTimeout());
Assert.Equal("Failed to invoke 'Stream' due to an error on the server. InvalidDataException: Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.", ex.Message);
Assert.Equal("Failed to invoke 'Stream' due to an error on the server. InvalidDataException: Error binding argument 1. Make sure that the types of the provided values match the types of the hub method being invoked.", ex.Message);
}
catch (Exception ex)
{
Expand Down
16 changes: 14 additions & 2 deletions ...ignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/GsonHubProtocol.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,13 @@ private ArrayList<Object> bindArguments(JsonArray argumentsToken, List<Type> par
if (paramTypes.size() >= 1) {
arguments = new ArrayList<>();
for (int i = 0; i < paramTypes.size(); i++) {
arguments.add(gson.fromJson(argumentsToken.get(i), paramTypes.get(i)));
Object obj;
try {
obj = gson.fromJson(argumentsToken.get(i), paramTypes.get(i));
} catch (Exception ex) {
throw new RuntimeException(String.format("Error binding argument %d. Make sure that the types of the provided values match the types of the method being invoked.", i + 1), ex);
}
arguments.add(obj);
}
}

Expand All @@ -251,7 +257,13 @@ private ArrayList<Object> bindArguments(JsonReader reader, List<Type> paramTypes
ArrayList<Object> arguments = new ArrayList<>();
while (reader.peek() != JsonToken.END_ARRAY) {
if (argCount < paramCount) {
arguments.add(gson.fromJson(reader, paramTypes.get(argCount)));
Object obj;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: what is the difference between this method and the other (lines 253 and 231)? They have identical signature with exception that one throws exception; is one of them not called at all?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One takes a JsonArray, the other takes a JsonReader.

The order of Json properties can be random, so if the array of arguments comes before the name of the method we're calling then we store the JsonArray to bind the arguments later, but if it's in the nicer order of (method, arguments) we can directly read the arguments from the JsonReader.

e.g.
{"target": "methodName", "arguments": [1, "blah"]}
vs.
{"arguments": [1, "blah"], "target": "methodName"}

try {
obj = gson.fromJson(reader, paramTypes.get(argCount));
} catch (Exception ex) {
throw new RuntimeException(String.format("Error binding argument %d. Make sure that the types of the provided values match the types of the method being invoked.", argCount + 1), ex);
}
arguments.add(obj);
} else {
reader.skipValue();
}
Expand Down
10 changes: 7 additions & 3 deletions ...lR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/GsonHubProtocolTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,8 @@ public void invocationBindingFailureWhenParsingIncorrectType() {

assertEquals(InvocationBindingFailureMessage.class, messages.get(0).getClass());
InvocationBindingFailureMessage invocationBindingFailureMessage = (InvocationBindingFailureMessage) messages.get(0);
assertEquals("java.lang.NumberFormatException: For input string: \"true\"", invocationBindingFailureMessage.getException().getMessage());
assertEquals("Error binding argument 1. Make sure that the types of the provided values match the types of the method being invoked.", invocationBindingFailureMessage.getException().getMessage());
assertEquals("java.lang.NumberFormatException: For input string: \"true\"", invocationBindingFailureMessage.getException().getCause().getMessage());
}

@Test
Expand All @@ -415,7 +416,8 @@ public void invocationBindingFailureStillReadsJsonPayloadAfterFailure() {

assertEquals(InvocationBindingFailureMessage.class, messages.get(0).getClass());
InvocationBindingFailureMessage invocationBindingFailureMessage = (InvocationBindingFailureMessage) messages.get(0);
assertEquals("java.lang.NumberFormatException: For input string: \"true\"", invocationBindingFailureMessage.getException().getMessage());
assertEquals("Error binding argument 1. Make sure that the types of the provided values match the types of the method being invoked.", invocationBindingFailureMessage.getException().getMessage());
assertEquals("java.lang.NumberFormatException: For input string: \"true\"", invocationBindingFailureMessage.getException().getCause().getMessage());
assertEquals("123", invocationBindingFailureMessage.getInvocationId());
}

Expand Down Expand Up @@ -444,7 +446,9 @@ public void invocationBindingFailureWhenParsingLocalDateTimeWithoutAppropriateTy
assertEquals(HubMessageType.INVOCATION_BINDING_FAILURE, message.getMessageType());
InvocationBindingFailureMessage failureMessage = (InvocationBindingFailureMessage) messages.get(0);

assertEquals("com.google.gson.JsonSyntaxException", failureMessage.getException().getClass().getName());
assertEquals("Error binding argument 1. Make sure that the types of the provided values match the types of the method being invoked.", failureMessage.getException().getMessage());
// Inner exception is inconsistent across versions/implementations, so we don't assert specifics
assertNotNull(failureMessage.getException().getCause());
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion src/SignalR/common/Protocols.Json/src/Protocol/JsonHubProtocol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -850,7 +850,7 @@ private static HubMessage BindInvocationMessage(string? invocationId, string tar
}
catch (Exception ex)
{
throw new InvalidDataException("Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.", ex);
throw new InvalidDataException($"Error binding argument {paramIndex + 1}. Make sure that the types of the provided values match the types of the hub method being invoked.", ex);
}
}
else
Expand Down
5 changes: 3 additions & 2 deletions src/SignalR/common/Protocols.MessagePack/src/Protocol/MessagePackHubProtocolWorker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,10 +303,11 @@ private static SequenceMessage CreateSequenceMessage(ref MessagePackReader reade
$"Invocation provides {argumentCount} argument(s) but target expects {parameterTypes.Count}.");
}

var i = 0;
try
{
var arguments = new object?[argumentCount];
for (var i = 0; i < argumentCount; i++)
for (; i < argumentCount; i++)
{
arguments[i] = DeserializeObject(ref reader, parameterTypes[i], "argument");
}
Expand All @@ -315,7 +316,7 @@ private static SequenceMessage CreateSequenceMessage(ref MessagePackReader reade
}
catch (Exception ex)
{
throw new InvalidDataException("Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.", ex);
throw new InvalidDataException($"Error binding argument {i + 1}. Make sure that the types of the provided values match the types of the hub method being invoked.", ex);
}
}

Expand Down
7 changes: 4 additions & 3 deletions src/SignalR/common/Protocols.NewtonsoftJson/src/Protocol/NewtonsoftJsonHubProtocol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -854,7 +854,7 @@ private static bool ReadArgumentAsType(JsonTextReader reader, IReadOnlyList<Type
}
catch (Exception ex)
{
throw new InvalidDataException("Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.", ex);
throw new InvalidDataException($"Error binding argument {paramIndex + 1}. Make sure that the types of the provided values match the types of the hub method being invoked.", ex);
}
}

Expand Down Expand Up @@ -908,9 +908,10 @@ private static SequenceMessage BindSequenceMessage(long? sequenceId)

var arguments = new object?[argCount];

var i = 0;
try
{
for (var i = 0; i < paramCount; i++)
for (; i < paramCount; i++)
{
var paramType = paramTypes[i];
arguments[i] = args[i].ToObject(paramType, PayloadSerializer);
Expand All @@ -920,7 +921,7 @@ private static SequenceMessage BindSequenceMessage(long? sequenceId)
}
catch (Exception ex)
{
throw new InvalidDataException("Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.", ex);
throw new InvalidDataException($"Error binding argument {i + 1}. Make sure that the types of the provided values match the types of the hub method being invoked.", ex);
}
}

Expand Down
10 changes: 5 additions & 5 deletions src/SignalR/common/SignalR.Common/test/Internal/Protocol/JsonHubProtocolTestsBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,13 +240,13 @@ public void ExtraItemsInMessageAreIgnored(string input)
[Theory]
[InlineData("{\"type\":1,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[]}", "Invocation provides 0 argument(s) but target expects 2.")]
[InlineData("{\"type\":1,\"arguments\":[], \"invocationId\":\"42\",\"target\":\"foo\"}", "Invocation provides 0 argument(s) but target expects 2.")]
[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.")]
[InlineData("{\"type\":1,\"invocationId\":\"42\",\"arguments\":[ \"abc\", \"xyz\"], \"target\":\"foo\"}", "Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.")]
[InlineData("{\"type\":1,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[ \"abc\", \"xyz\"]}", "Error binding argument 1. Make sure that the types of the provided values match the types of the hub method being invoked.")]
[InlineData("{\"type\":1,\"invocationId\":\"42\",\"arguments\":[ \"abc\", \"xyz\"], \"target\":\"foo\"}", "Error binding argument 1. Make sure that the types of the provided values match the types of the hub method being invoked.")]
[InlineData("{\"type\":4,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[]}", "Invocation provides 0 argument(s) but target expects 2.")]
[InlineData("{\"type\":4,\"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.")]
[InlineData("{\"type\":4,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[ \"abc\", \"xyz\"]}", "Error binding argument 1. Make sure that the types of the provided values match the types of the hub method being invoked.")]
[InlineData("{\"type\":1,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[1,\"\",{\"1\":1,\"2\":2}]}", "Invocation provides 3 argument(s) but target expects 2.")]
[InlineData("{\"type\":1,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[1,[1]]}", "Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.")]
[InlineData("{\"type\":1,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[1,[]]}", "Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.")]
[InlineData("{\"type\":1,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[1,[1]]}", "Error binding argument 2. Make sure that the types of the provided values match the types of the hub method being invoked.")]
[InlineData("{\"type\":1,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[1,[]]}", "Error binding argument 2. 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);
Expand Down
4 changes: 2 additions & 2 deletions src/SignalR/common/SignalR.Common/test/Internal/Protocol/MessagePackHubProtocolTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,14 +373,14 @@ protected void TestInvalidMessageDate(InvalidMessageData testData)
new InvalidMessageData("InvocationArgumentArrayNotAnArray", new byte[] { 0x95, 1, 0x80, 0xa3, (byte)'a', (byte)'b', (byte)'c', 0xa3, (byte)'x', (byte)'y', (byte)'z', 42 }, "Reading array length for 'arguments' failed."),
new InvalidMessageData("InvocationArgumentArraySizeMismatchEmpty", new byte[] { 0x95, 1, 0x80, 0xa3, (byte)'a', (byte)'b', (byte)'c', 0xa3, (byte)'x', (byte)'y', (byte)'z', 0x90 }, "Invocation provides 0 argument(s) but target expects 1."),
new InvalidMessageData("InvocationArgumentArraySizeMismatchTooLarge", new byte[] { 0x95, 1, 0x80, 0xa3, (byte)'a', (byte)'b', (byte)'c', 0xa3, (byte)'x', (byte)'y', (byte)'z', 0x92, 0xa1, (byte)'a', 0xa1, (byte)'b' }, "Invocation provides 2 argument(s) but target expects 1."),
new InvalidMessageData("InvocationArgumentTypeMismatch", new byte[] { 0x95, 1, 0x80, 0xa3, (byte)'a', (byte)'b', (byte)'c', 0xa3, (byte)'x', (byte)'y', (byte)'z', 0x91, 42 }, "Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked."),
new InvalidMessageData("InvocationArgumentTypeMismatch", new byte[] { 0x95, 1, 0x80, 0xa3, (byte)'a', (byte)'b', (byte)'c', 0xa3, (byte)'x', (byte)'y', (byte)'z', 0x91, 42 }, "Error binding argument 1. Make sure that the types of the provided values match the types of the hub method being invoked."),

// StreamInvocationMessage
new InvalidMessageData("StreamInvocationArgumentArrayMissing", new byte[] { 0x94, 4, 0x80, 0xa3, (byte)'a', (byte)'b', (byte)'c', 0xa3, (byte)'x', (byte)'y', (byte)'z' }, "Reading array length for 'arguments' failed."), // array is missing
new InvalidMessageData("StreamInvocationArgumentArrayNotAnArray", new byte[] { 0x95, 4, 0x80, 0xa3, (byte)'a', (byte)'b', (byte)'c', 0xa3, (byte)'x', (byte)'y', (byte)'z', 42 }, "Reading array length for 'arguments' failed."), // arguments isn't an array
new InvalidMessageData("StreamInvocationArgumentArraySizeMismatchEmpty", new byte[] { 0x95, 4, 0x80, 0xa3, (byte)'a', (byte)'b', (byte)'c', 0xa3, (byte)'x', (byte)'y', (byte)'z', 0x90 }, "Invocation provides 0 argument(s) but target expects 1."), // array is missing elements
new InvalidMessageData("StreamInvocationArgumentArraySizeMismatchTooLarge", new byte[] { 0x95, 4, 0x80, 0xa3, (byte)'a', (byte)'b', (byte)'c', 0xa3, (byte)'x', (byte)'y', (byte)'z', 0x92, 0xa1, (byte)'a', 0xa1, (byte)'b' }, "Invocation provides 2 argument(s) but target expects 1."), // argument count does not match binder argument count
new InvalidMessageData("StreamInvocationArgumentTypeMismatch", new byte[] { 0x95, 4, 0x80, 0xa3, (byte)'a', (byte)'b', (byte)'c', 0xa3, (byte)'x', (byte)'y', (byte)'z', 0x91, 42 }, "Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked."), // argument type mismatch
new InvalidMessageData("StreamInvocationArgumentTypeMismatch", new byte[] { 0x95, 4, 0x80, 0xa3, (byte)'a', (byte)'b', (byte)'c', 0xa3, (byte)'x', (byte)'y', (byte)'z', 0x91, 42 }, "Error binding argument 1. Make sure that the types of the provided values match the types of the hub method being invoked."), // argument type mismatch
}.ToDictionary(t => t.Name);

public static IEnumerable<object[]> ArgumentBindingErrorNames => ArgumentBindingErrors.Keys.Select(name => new object[] { name });
Expand Down
Loading