Fail client result if client can't parse invocation argument(s) (#47003)

Co-authored-by: Brennan Conroy <brecon@microsoft.com>
dotnet · Mar 10, 2023 · e2188b2 · e2188b2
1 parent 7124b90
commit e2188b2
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 0 deletions.
diff --git a/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs b/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs
@@ -986,6 +986,11 @@ private async Task SendWithLock(ConnectionState expectedConnectionState, HubMess
                 // The server can't receive a response, so we just drop the message and log
                 // REVIEW: Is this the right approach?
                 Log.ArgumentBindingFailure(_logger, bindingFailure.InvocationId, bindingFailure.Target, bindingFailure.BindingFailure.SourceException);
+
+                if (!string.IsNullOrEmpty(bindingFailure.InvocationId))
+                {
+                    await SendWithLock(connectionState, CompletionMessage.WithError(bindingFailure.InvocationId, "Client failed to parse argument(s)."), cancellationToken: default).ConfigureAwait(false);
+                }
                 break;
             case InvocationMessage invocation:
                 Log.ReceivedInvocation(_logger, invocation.InvocationId, invocation.Target, invocation.Arguments);

diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.Protocol.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.Protocol.cs
@@ -852,6 +852,31 @@ public async Task ClientResultReturnsErrorIfNoResultFromClient()
             }
         }
 
+        [Fact]
+        public async Task ClientResultReturnsErrorIfCannotParseArgument()
+        {
+            var connection = new TestConnection();
+            var hubConnection = CreateHubConnection(connection);
+            try
+            {
+                await hubConnection.StartAsync().DefaultTimeout();
+
+                // No result provided
+                hubConnection.On("Result", (string _) => 1);
+
+                await connection.ReceiveTextAsync("{\"type\":1,\"invocationId\":\"1\",\"target\":\"Result\",\"arguments\":[15]}\u001e").DefaultTimeout();
+
+                var invokeMessage = await connection.ReadSentTextMessageAsync().DefaultTimeout();
+
+                Assert.Equal("{\"type\":3,\"invocationId\":\"1\",\"error\":\"Client failed to parse argument(s).\"}", invokeMessage);
+            }
+            finally
+            {
+                await hubConnection.DisposeAsync().DefaultTimeout();
+                await connection.DisposeAsync().DefaultTimeout();
+            }
+        }
+
         [Fact]
         public async Task ClientResultCanReturnNullResult()
         {

diff --git a/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HubConnection.java b/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HubConnection.java
@@ -469,6 +469,11 @@ private void ReceiveLoop(ByteBuffer payload)
                 case INVOCATION_BINDING_FAILURE:
                     InvocationBindingFailureMessage msg = (InvocationBindingFailureMessage)message;
                     logger.error("Failed to bind arguments received in invocation '{}' of '{}'.", msg.getInvocationId(), msg.getTarget(), msg.getException());
+
+                    if (msg.getInvocationId() != null) {
+                        sendHubMessageWithLock(new CompletionMessage(null, msg.getInvocationId(),
+                            null, "Client failed to parse argument(s)."));
+                    }
                     break;
                 case INVOCATION:
                     InvocationMessage invocationMessage = (InvocationMessage) message;

diff --git a/...java/signalr/test/src/main/java/com/microsoft/signalr/HubConnection.ReturnResultTest.java b/...java/signalr/test/src/main/java/com/microsoft/signalr/HubConnection.ReturnResultTest.java
@@ -429,4 +429,22 @@ public void clientResultHandlerDoesNotBlockOtherHandlers() {
         String expected = "{\"type\":3,\"invocationId\":\"1\",\"result\":\"bob\"}" + RECORD_SEPARATOR;
         assertEquals(expected, TestUtils.byteBufferToString(message));
     }
+
+    @Test
+    public void clientResultReturnsErrorIfCannotParseArgument() {
+        MockTransport mockTransport = new MockTransport();
+        HubConnection hubConnection = TestUtils.createHubConnection("http://example.com", mockTransport);
+
+        hubConnection.onWithResult("inc", (i) -> {
+            return Single.just("bob");
+        }, Integer.class);
+
+        hubConnection.start().timeout(30, TimeUnit.SECONDS).blockingAwait();
+        SingleSubject<ByteBuffer> sendTask = mockTransport.getNextSentMessage();
+        mockTransport.receiveMessage("{\"type\":1,\"invocationId\":\"1\",\"target\":\"inc\",\"arguments\":[\"not int\"]}" + RECORD_SEPARATOR);
+
+        ByteBuffer message = sendTask.timeout(30, TimeUnit.SECONDS).blockingGet();
+        String expected = "{\"type\":3,\"invocationId\":\"1\",\"error\":\"Client failed to parse argument(s).\"}" + RECORD_SEPARATOR;
+        assertEquals(expected, TestUtils.byteBufferToString(message));
+    }
 }